Add Revcontent Adapter #4654

AzizSaleh · 2019-12-19T20:46:16Z

Type of change

Bugfix
Feature
New bidder adapter
Code style update (formatting, local variables)
Refactoring (no functional changes, no api changes)
Build related changes
CI related changes
Does this change affect user-facing APIs or examples documented on http://prebid.org?
Other

Description of change

Adding RevContent Bidder

test parameters for validating bids

{
  bidder: 'revcontent',
  params: {
      apiKey: '8a33sdfsdfdsfsdfssss544f8sdfsdfsdfd3b1c',  // Required
      userId: 69565,                                      // Required
      widgetId: 599995,                                   // Optional
      domain: 'test.com',                                 // Optional - Default referral hostname
  }
}

Test page: /integrationExamples/gpt/revcontent_example.html

contact email of the adapter’s maintainer: aziz@revcontent.com
[x] official adapter submission

For any changes that affect user-facing APIs or example code documented on http://prebid.org, please provide:

A link to a PR on the docs repo at https://github.com/prebid/prebid.github.io/: RevContent Bidder Documentation prebid.github.io#1705

robertrmartinez · 2019-12-19T20:47:34Z

@AzizSaleh Now we're talkin!

Thanks, I will review and make some official comments sometime today / tomorrow.

We will be able to get this into the next prebid release!

AzizSaleh · 2019-12-19T20:48:39Z

Thanks man! Much appreciated.

robertrmartinez · 2020-01-02T18:12:25Z

modules/revcontentBidAdapter.js

+  code: BIDDER_CODE,
+  supportedMediaTypes: ['banner', 'native', 'video'],
+  isBidRequestValid: function (bid) {
+    return (typeof bid.params.apiKey !== 'undefined' && typeof bid.params.userId !== 'undefined' && bid.hasOwnProperty('nativeParams'));


so no matter what each bid needs nativeParams ?

Is revcontent only doing native ads?

Or am I missing something?

Yep only native at this time. I am going to update supportedMediaTypes to reflect.

robertrmartinez · 2020-01-02T18:16:59Z

modules/revcontentBidAdapter.js

+    }
+
+    if (typeof domain === 'undefined') {
+      domain = extractHostname(refererInfo);


it may be possible that refererInfo is undefined at this point.

Which I believe could throw Cannot read property 'indexOf' of undefined inside the function extractHostname

May be a good idea to default this or skip this if refererInfo is not defined here.

I am going to update the actual function to return an empty string in such event.

robertrmartinez · 2020-01-02T18:18:00Z

modules/revcontentBidAdapter.js

+      domain = extractHostname(refererInfo);
+    }
+
+    var endpoint = '//' + host + '/rtb?apiKey=' + apiKey + '&userId=' + userId;


For Prebid 3.0 we now require only HTTPS connections.

I am going to update it so that it is secure all the time.

robertrmartinez · 2020-01-02T18:19:21Z

modules/revcontentBidAdapter.js

+      endpoint = endpoint + '&widgetId=' + widgetId;
+    }
+
+    let secure = location.protocol === 'https:';


This looks to be a param passed to your ad exchange.

As noted above, we are requiring that all connections to be HTTPS now.

So not sure if you want to update this to always be true

robertrmartinez · 2020-01-02T18:21:17Z

modules/revcontentBidAdapter.js

+    let secure = location.protocol === 'https:';
+
+    const imp = validBidRequests.map((bid, id) => {
+      if (bid.hasOwnProperty('nativeParams')) {


seems like (as mentioned above) nativeParams will always be on your bids since you are requiring it in validateBidRequests

robertrmartinez · 2020-01-02T18:23:32Z

modules/revcontentBidAdapter.js

+
+            return asset;
+          }
+        }).filter(Boolean);


nice usage of filter(Boolean)

To be fair, I copied that from the openRtb adapter implementation (since we are also using open rtb on the backend as well).

robertrmartinez · 2020-01-02T18:31:54Z

modules/revcontentBidAdapter.js

+
+export const spec = {
+  code: BIDDER_CODE,
+  supportedMediaTypes: ['banner', 'native', 'video'],


seems like we are only supporting native as of now since nativeParams is required on every bid?

robertrmartinez · 2020-01-02T18:33:25Z

@AzizSaleh Couple of questions.

robertrmartinez · 2020-01-02T18:44:23Z

When testing the adapter I noticed the onBidWon imp trackers you are firing are returning a access-control-allow-origin which is not correct (Probably only because my test page utilzes ports) but wanted you to be aware?

Which then throws a:

Access to XMLHttpRequest at 'https://trends-s0.revcontent.com/imp.php' from origin 'http://www.rubitest.com:8000' has been blocked by CORS policy: The 'Access-Control-Allow-Origin' header has a value 'http://www.rubitest.com' that is not equal to the supplied origin.

Probably works fine for normal hostnames but wanted you to be aware.

robertrmartinez · 2020-01-02T18:53:42Z

modules/revcontentBidAdapter.js

+    var host = extractHostname(winUrl);
+
+    ajax(winUrl + '&viewed=1', null, {withCredentials: true});
+    ajax('https://' + host + '/imp.php', null, 'v=' + encodeURIComponent(encodeURIComponent(getQueryVariable('d', winUrl))) + '&i=' + encodeURIComponent(window.location.href), {method: 'POST', contentType: 'application/x-www-form-urlencoded'});


While we do allow listening and firing imp trackers, can you explain the necessity to fire TWO imp trackers here?

We have allowed adapters to fire one in previous PR's, but wanted to know this use case and see if it can be updated to fire a single ajax call?

Those are for different uses at the moment, we are working on reducing them into 1 call but to have accurate reporting we will need them both atm.

@AzizSaleh Ok, so I spoke with the Prebid Org team and while we are a little hesitant as we want to limit things like this, we will allow it.

So whenever your team gets around to reducing it to 1 please make the corresponding change here!

It may become a restricted thing in a future version of Prebid (Only allowing one track to fire per bidWon) but I will let you know ahead of time.

AzizSaleh · 2020-01-03T19:46:20Z

When testing the adapter I noticed the onBidWon imp trackers you are firing are returning a access-control-allow-origin which is not correct (Probably only because my test page utilzes ports) but wanted you to be aware?

Which then throws a:
Access to XMLHttpRequest at 'https://trends-s0.revcontent.com/imp.php' from origin 'http://www.rubitest.com:8000' has been blocked by CORS policy: The 'Access-Control-Allow-Origin' header has a value 'http://www.rubitest.com' that is not equal to the supplied origin.
Probably works fine for normal hostnames but wanted you to be aware.

Yep that also happened to me while testing locally. Had to upload it live to see actual results:

https://feudfun.com/test21/revcontent_example.php

…tractHostname method

AzizSaleh · 2020-01-03T19:57:04Z

@robertrmartinez I made a few updates, however, when doing a git pull (force of habit before committing) seems like pakage-lock.json. If everything looks fine, I can re-create the branch like I did last time to avoid that.

robertrmartinez

Ok looks good!

AzizSaleh · 2020-01-07T15:17:55Z

@robertrmartinez Greatly appreciate the +1. Quick question about the process, when should we expect a merge/deploy and what other steps do we need to do (more of timeline process question).

robertrmartinez · 2020-01-07T19:00:12Z

@AzizSaleh We will be releasing a new Prebid Version later today or tomorrow, so I will be sure to merge this into master before that!

So your adapter will officially be in Prebid Versions 3.2.0 and later!

robertrmartinez · 2020-01-07T19:34:00Z

@AzizSaleh Well I am really sorry but I dropped the ball!

I just now realized that you do not even have a test file.

We require that each adapter has a spec file which institutes 80% test coverage!

I went to check the test coverage for your adapter and realized it did not have a file at all!

Can you please add a file and get it up to 80%.

AzizSaleh · 2020-01-08T00:11:44Z

@robertrmartinez Added the test file + removed a missed console.log on actual adapter.

…idVersion1.2.0 * 'master' of https://github.com/prebid/Prebid.js: (22 commits) fix lint errors in unit test file (prebid#4702) Add Revcontent Adapter (prebid#4654) Changed data structure in Platform One Analytic Adapter (prebid#4647) increment pre version Prebid 3.2.0 Release Add static API option to the consentManagementUsp module. (prebid#4685) replace all xhr stubs with global xhr stub to prevent all requests (prebid#4687) Add CCPA us_privacy support to spotxBidAdapter (prebid#4689) ucfunnel adapter support CCPA and remove utils.js in adapter (prebid#4541) freewheelSSPBidAdapter (prebid#4645) Add CCPA support to Beachfront adapter (prebid#4673) add seedingAlliance Adapter (prebid#4614) Changed analytics data structure in YuktaMedia Analytic Adapter (prebid#4659) Add eplanning adapter for prebid 3.0 compliant and CCPA and GDPR support (prebid#4643) Bidder schain support (prebid#4551) Added CCPA support and GDPR compliance to Cedato adapter (prebid#4683) pass us privacy consent string to request (prebid#4581) Prebid 3 Admixer (prebid#4615) Pass uspConsent in bidRequest (prebid#4675) Advertly: New Bidder Adapter Submission (prebid#4496) ...

* Initial commit * Update * Update js * Update file name * Making it work (at least getting a response) * Update native logic * Updates to bidder * Update prebidder * Remove debugs * Updates * Add new templates * Update templates * Update email address * Resolve conflict * Resolve conflict * Rebase from master * Rebase from master * Add Revcontent Adapter * Force https, update supportedMediaTypes and add default return for extractHostname method * Add test file + remove console.log statement

AzizSaleh added 18 commits November 13, 2019 12:48

Initial commit

ea58f26

Update

67ae89c

Update js

6768104

Update file name

fbf2ee0

Making it work (at least getting a response)

5ccb6fe

Update native logic

c1a4264

Updates to bidder

68ddfa3

Update prebidder

11a9c8a

Remove debugs

bb3beb3

Updates

eb87938

Add new templates

ab2d929

Update templates

17181a1

Update email address

c60dd3e

Resolve conflict

41ff091

Resolve conflict

89794ff

Rebase from master

c2cc2b1

Rebase from master

c6980d5

Add Revcontent Adapter

4e00e8a

robertrmartinez self-requested a review December 19, 2019 20:46

mike-chowla assigned robertrmartinez Dec 30, 2019

robertrmartinez requested changes Jan 2, 2020

View reviewed changes

robertrmartinez reviewed Jan 2, 2020

View reviewed changes

Merge branch 'master' of https://github.com/RevContent/Prebid.js

72e48ce

Force https, update supportedMediaTypes and add default return for ex…

41b8b54

…tractHostname method

robertrmartinez approved these changes Jan 6, 2020

View reviewed changes

Add test file + remove console.log statement

f043536

robertrmartinez merged commit 8c42f02 into prebid:master Jan 8, 2020

jsnellbaker mentioned this pull request Jan 8, 2020

fix lint errors in unit test file #4702

Merged

1 task

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add Revcontent Adapter #4654

Add Revcontent Adapter #4654

AzizSaleh commented Dec 19, 2019 •

edited

Loading

robertrmartinez commented Dec 19, 2019

AzizSaleh commented Dec 19, 2019

robertrmartinez Jan 2, 2020

AzizSaleh Jan 3, 2020

robertrmartinez Jan 2, 2020

AzizSaleh Jan 3, 2020

robertrmartinez Jan 2, 2020

AzizSaleh Jan 3, 2020

robertrmartinez Jan 2, 2020

robertrmartinez Jan 2, 2020

robertrmartinez Jan 2, 2020

AzizSaleh Jan 3, 2020

robertrmartinez Jan 2, 2020

robertrmartinez commented Jan 2, 2020

robertrmartinez commented Jan 2, 2020

robertrmartinez Jan 2, 2020

AzizSaleh Jan 3, 2020

robertrmartinez Jan 6, 2020

AzizSaleh commented Jan 3, 2020

AzizSaleh commented Jan 3, 2020

robertrmartinez left a comment

AzizSaleh commented Jan 7, 2020

robertrmartinez commented Jan 7, 2020

robertrmartinez commented Jan 7, 2020

AzizSaleh commented Jan 8, 2020

Add Revcontent Adapter #4654

Add Revcontent Adapter #4654

Conversation

AzizSaleh commented Dec 19, 2019 • edited Loading

Type of change

Description of change

robertrmartinez commented Dec 19, 2019

AzizSaleh commented Dec 19, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

robertrmartinez commented Jan 2, 2020

robertrmartinez commented Jan 2, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AzizSaleh commented Jan 3, 2020

AzizSaleh commented Jan 3, 2020

robertrmartinez left a comment

Choose a reason for hiding this comment

AzizSaleh commented Jan 7, 2020

robertrmartinez commented Jan 7, 2020

robertrmartinez commented Jan 7, 2020

AzizSaleh commented Jan 8, 2020

AzizSaleh commented Dec 19, 2019 •

edited

Loading